Add upstream-shaped scoped introspection plugin - #1763
Conversation
45cbb07 to
658736e
Compare
30c480a to
50cbb64
Compare
658736e to
f7c63ec
Compare
50cbb64 to
4d35ddb
Compare
|
Review complete. 🟡 1 medium 💬 Inline comments (1)
This PR vendors a scoped PostgreSQL introspection engine (dependency-closure CTE, ACL parsing, schema validation, watch/cache lifecycle) into a new graphile-scoped-introspection package and wires it through graphile-settings, the graphql/env option merge, the server middleware, and a new perf-harness benchmark suite with its own CI batch. The change is largely additive and well-tested, but one medium finding surfaced in the core plugin.
The primary defect: Reviewed commit: 4d35ddb |
There was a problem hiding this comment.
Adds a CNC-owned scoped PostgreSQL introspection replacement (graphile-scoped-introspection), wiring it into graphile-settings, graphql/env config parsing, the GraphQL server middleware, and a new perf-harness benchmark package.
Key findings
- 🟡
capabilityExtensionsschema rejected byassertScopedNamespaces— PgIntrospectionPlugin.ts:299
| allowedSchemas: [ | ||
| ...new Set([...requiredSchemas, ...dependencySchemas, 'pg_catalog']), | ||
| ], |
There was a problem hiding this comment.
🟡 bug · medium
capabilityExtensions schema rejected by assertScopedNamespaces
capabilityExtensions pulls the extension's schema into scoped_namespaces (scopedIntrospection.ts:328-330), but allowedSchemas (PgIntrospectionPlugin.ts:299-301) never includes it, so assertScopedNamespaces throws crossed into unapproved dependency schema and the gather hard-fails. Setting introspectionCapabilityExtensions without also listing the extension's schema in introspectionDependencySchemas breaks schema build entirely, making the option ineffective for its stated purpose.
📋 Prompt for AI Agents
In graphile/graphile-scoped-introspection/src/upstream/graphile-build-pg/PgIntrospectionPlugin.ts around lines 294-303, extend the allowedSchemas array to also include the namespaces of the configured capability extensions (resolve each capability extension's extnamespace from the introspected extensions/namespaces), so that assertScopedNamespaces in scopedValidation.ts does not reject the extension's schema. Currently capabilityExtensions (line 296) adds the extension's schema to scoped_namespaces in the SQL CTE, but allowedSchemas (lines 299-301) omits it, causing a hard 'crossed into unapproved dependency schema' failure whenever a capability extension lives in a schema not already listed in introspectionDependencySchemas.
4d35ddb to
e40749d
Compare
Summary
This is an upstream-shaped, opt-in alternative to #1719. It copies the exact Crystal release sources pinned by CNC (
[email protected],[email protected]) and keeps the upstream package boundaries while adding a scoped query and replacement introspection plugin.Unset/
falseuses untouched upstream stock introspection.GRAPHILE_SCOPED_INTROSPECTION=truedynamically installsScopedIntrospectionPreset;GRAPHILE_SCOPED_INTROSPECTION_JITindependently controls the introspection session JIT setting and defaults tofalse.Architecture and configuration
pg-introspection: parameterized recursive schema/dependency-closure query,all/dependency-closurecatalog type policy, and extension capability retention.graphile-build-pg: complete CNC-ownedPgScopedIntrospectionPlugin, upstream lifecycle/watch/entity behavior, namespace and missing-type fail-closed guards, and pinned upstream contract sentinel.preset.gather, keyed by PostgreSQL service name. PG service objects are no longer mutated or extended with CNC-only fields.gather.pgScopedIntrospectionand keeps JIT inpgSettingsForIntrospection.ConstructivePresetremains unchanged.Equivalent direct configuration:
Presence of a service entry enables scoped introspection for that service. Services without an entry remain stock, mixed stock/scoped services are supported, and an unknown configured service fails closed.
Benchmark arms differ only by upstream
PgIntrospectionPlugin/stock query versusScopedIntrospectionPreset/scoped query. They do not loadConstructivePreset, retirement/disposers, governor/cache limits, routing, or admission plugins.Correctness coverage
The real PostgreSQL dependency-closure fixture covers cross-schema enum/domain/composite/range/multirange types, function signatures, ordinary and
pg_trgmindexes, extension metadata, a cross-schema foreign key, and inheritance direction. It proves the required dependency parent is retained while a reverse parent-to-child relationship does not pull an unrelated child into the closure. No production query-algorithm change was needed for this configuration migration.Additional contracts cover mixed stock/scoped services, stock replacement equivalence, persistent gather cache behavior, unknown-service rejection, invalid dependency schemas, missing-type rejection, schema equality, and runtime execution.
Catalog benchmark
The scoped-only benchmark reuses the generic fresh-process runner/protocol/report. It uses a fixed 65-table root surface plus dependency and unrelated noise schemas, scalable catalog noise, identical explicit JIT settings, public gather-hook entity counts, and a separate query-only worker. Every arm used 10 fresh-process samples and all 240 worker PIDs were unique.
Environment: macOS arm64, Node v22.22.0, PostgreSQL 18.4 in a dedicated local container. PostgreSQL catalog cache is labeled
shared-server-not-reset; this is not a pristine-cache cold-start claim.JIT off (primary)
Negative percentages mean scoped reduced the metric.
At large scale, parsed classes fell from 64,220 to 270 and median heap fell from 839.6 MiB to 46.7 MiB. The small fixture shows why the feature remains opt-in.
JIT diagnostic
With JIT enabled, compilation dominates the recursive query at small/medium scale. The primary comparison therefore uses identical JIT-off sessions, matching CNC's scoped default. The benchmark reports the requested and observed JIT value for every run.
Stock/scoped schema hashes matched at every size and every runtime verification query passed:
d31e8b1c2d9cab3d1e67dcbc734e5addd237637028089221cf5f014b9ede4aa58c4e2e03008c98295978712461b799d23957ae89071326a16421ac4a7d0b199641c6ed8feb087766680ab1dc4d833e5082dbf69322b5fb2a118515e7c7ff9db0Reproduction
Fixture preparation never replaces or drops schemas. The large synthetic result demonstrates scale behavior; it is not a claim about typical production gains.
Validation
Passing:
graphile-scoped-introspection: 30 tests / 8 suites, including 6 real PostgreSQL dependency-closure casesgraphile-settingswiring/JIT contracts: 3 testspnpm install --frozen-lockfilegit diff --checkand PR-wide format-only diff auditNo database URL or password appears in benchmark JSON output.